gh-125053: Document that ob_mutex must only be used via critical section API#144599
gh-125053: Document that ob_mutex must only be used via critical section API#144599overlorde wants to merge 5 commits intopython:mainfrom
Conversation
|
All commit authors signed the Contributor License Agreement. |
bedevere-app
bot
added
awaiting review
docs
overlorde
force-pushed
the
doc-ob-mutex-warning
branch
from
0d37059 to
c1f0568
Compare
…l section API Add a warning in the free-threading extensions howto explaining that PyObject.ob_mutex is reserved for the critical section API and must not be locked directly with PyMutex_Lock, as this can cause deadlocks. Extension authors who need their own lock should add a separate PyMutex field to their object struct. Also add an ob_mutex member entry under PyObject in the C API reference (Doc/c-api/structures.rst) with a cross-reference to the howto.
overlorde
force-pushed
the
doc-ob-mutex-warning
branch
from
c1f0568 to
8c202f0
Compare
|
Hello, hope this works, claiming this issue. Thanks! |
| :c:type:`PyMutex` is very lightweight — it is only one byte — so there is | ||
| negligible cost to having an additional one:: | ||
|
|
||
| /* WRONG — do not lock ob_mutex directly */ |
There was a problem hiding this comment.
I don't think the example is needed, the explanation text seems sufficient.
| same mutex can cause deadlocks, because the critical section implementation | ||
| may suspend and release its locks when contention is detected. |
There was a problem hiding this comment.
because the critical section implementation may suspend and release its locks when contention is detected
I don't think this is related to why it may deadlock. I'd remove everything from ", because the ..." onwards.
| For example, the garbage collector or other interpreter internals may enter a | ||
| critical section on your object. If your code holds ``ob_mutex`` directly at | ||
| that point, a deadlock can occur. |
There was a problem hiding this comment.
I don't think the garbage collector ever acquires critical sections on objects. Maybe just:
Even if your own code never uses critical sections on a particular object type,
CPython internals may use the critical section API on any Python object.
If your code holds ob_mutex directly at that point, a deadlock can occur.
There was a problem hiding this comment.
If your code holds ob_mutex directly at that point, a deadlock can occur.
It sounds redundant with the above warning. Is it worth it to say it again?
There was a problem hiding this comment.
Yeah, seems fine to get rid of that as well to me.
| For example, the garbage collector or other interpreter internals may enter a | ||
| critical section on your object. If your code holds ``ob_mutex`` directly at | ||
| that point, a deadlock can occur. |
There was a problem hiding this comment.
If your code holds ob_mutex directly at that point, a deadlock can occur.
It sounds redundant with the above warning. Is it worth it to say it again?
| :c:type:`PyMutex` is very lightweight — it is only one byte — so there is | ||
| negligible cost to having an additional one. |
There was a problem hiding this comment.
I'm not sure that it's worth it to say that it only consumes 1 byte in the object structure. Because of alignment, the addition can take more bytes.
4 participants
Summary
ob_mutex)" subsection to the free-threading extensions howto (Doc/howto/free-threading-extensions.rst) warning thatob_mutexis reserved for the critical section API and must not be locked directly withPyMutex_Lockob_mutexmember entry underPyObjectin the C API reference (Doc/c-api/structures.rst) with a cross-reference to the howtoInclude code examples showing wrong (direct lock) vs. right (critical section or separate mutex) approachesCloses #125053
Test plan
make htmlinDoc/builds with no warningsstructures.htmltofree-threading-extensions.html#per-object-locksrenders correctlyInclude/object.h,Include/cpython/pylock.h,Include/internal/pycore_critical_section.h)ob_mutexshould only be used with the critical section API #125053📚 Documentation preview 📚: